Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add baseline functionality to ktlint #707

Merged
merged 11 commits into from
Nov 18, 2020

Conversation

joshafeinberg
Copy link
Contributor

Adds in a new flag, baseline, that allows you to specify a baseline to check again when running your report.

The functionality is similar to Android lint's baseline. It creates a snapshot file of the current errors in the project on the first run using the checkstyle reporter. On all following runs it checks against that baseline file and only reports errors that do not exist inside that file.

@shashachu
Copy link
Contributor

I wonder if it would be cleaner to make your own reporter rather than rely on checkstyle. Seems like a non-obvious dependency that changing checkstyle could potentially break this functionality.

Also - it looks like the --baseline param both generates or loads the baseline file? Maybe less confusing to break them into separate commands.

@shashachu
Copy link
Contributor

We'd also want to build a set of tests.

@joshafeinberg
Copy link
Contributor Author

I wonder if it would be cleaner to make your own reporter rather than rely on checkstyle. Seems like a non-obvious dependency that changing checkstyle could potentially break this functionality.

Fair point, just wasn't sure if that would be too large of a change. I'll add in a new reporter tonight.

Also - it looks like the --baseline param both generates or loads the baseline file? Maybe less confusing to break them into separate commands.

That is how it works with Android lint so I copied that model. If it exists then it loads otherwise it generates so that it can be run against the next time. I'm not sure if it makes sense to have two flags when you wouldn't know if you had a baseline file or not though happy to add in if you prefer

We'd also want to build a set of tests.

I'll add some tonight as well where possible

@joshafeinberg
Copy link
Contributor Author

joshafeinberg commented Feb 28, 2020

So now there is an interesting issue - I can't have test resources that fail because they don't follow ktlint but I need them not to so that the tests pass. Any thoughts @shashachu?

Edit: fun fact, I tried adding the baseline flag to the build.gradle and once it generates the baseline for the project it builds fine. Not sure how we'd feel about using the functionality in the build script though

@jeffmcnd
Copy link

@shashachu how does this PR look now? This is a much needed feature!

@LionZXY
Copy link

LionZXY commented Sep 16, 2020

Any update? Need this feature

@splundid
Copy link

Currently this prevents our team from adopting ktlint, the feature is critical for large projects with huge poorly formatted legacy codebase

@shashachu shashachu merged commit dbfbfc7 into pinterest:master Nov 18, 2020
@emrisb
Copy link

emrisb commented Jan 26, 2021

I implemented ktlint in old multi module project with baseline, but it generates a baseline file for each module. Is there any way to generate one baseline file for all modules?

@joshfeinberg-ah
Copy link

I implemented ktlint in old multi module project with baseline, but it generates a baseline file for each module. Is there any way to generate one baseline file for all modules?

Not in how this is intended to work. Feel free to file a feature request

@eboudrant
Copy link

Great feature @joshfeinberg. I have a question, if you run ktlint with the baseline and the -F option, is it supposed to fix the code noted in the baseline? I would expect it to not fix code in baseline but it seems it fix all code. That would be great for having a command to format the new code only.

@joshafeinberg
Copy link
Contributor Author

Great feature @joshfeinberg. I have a question, if you run ktlint with the baseline and the -F option, is it supposed to fix the code noted in the baseline? I would expect it to not fix code in baseline but it seems it fix all code. That would be great for having a command to format the new code only.

The baseline functionality does not affect the formatter option at all. You can configure ktlint to just run on files currently in your git stage and that should solve this issue. Feel free to file a feature request on ktlint if you have a reason to specifically want a feature to not change stuff in the baseline

@eboudrant
Copy link

I see. We use git stage. The reason was to prevent reformatting an entire file when you only touch few lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants